Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

deepin pr auto review

★ 总体评分：35分

■ 【总体评价】

代码将进程启动逻辑下沉以解决重复拉起问题，但引入了严重的不安全临时文件使用漏洞和未解决的竞态条件。
逻辑正确但因存在高危符号链接劫持风险及中危状态校验缺失扣65分

■ 【详细分析】

• 1.语法逻辑存在错误✕

代码将服务启动从 getPkgMan() 移至 main() 函数，解决了重复触发的问题，但注释中写“避免硬编码 sleep 导致的竞态问题”，实际代码中仍然保留了 std::this_thread::sleep_for(1s);，注释与实现存在严重矛盾，且竞态问题并未真正解决，服务可能未就绪即进行后续操作。
潜在问题：竞态条件依然存在；注释与代码实现不一致导致维护者误解。
建议：移除误导性注释，实现基于 socket 文件状态或连接重试的真实就绪检测机制。

• 2.代码质量一般✕

职责分离做得较好，getPkgMan() 不再承担启动进程的副作用。但使用固定时间休眠作为同步手段是极差的实践，属于典型的反模式代码。
潜在问题：硬编码的 1 秒休眠在不同负载的机器上表现不一致，可能导致启动过慢或失败。
建议：引入带有超时的轮询机制，检查 socket 文件是否可连接，替代盲等。

• 3.代码性能存在性能问题✕

将休眠从可能被多次调用的 getPkgMan() 移至仅执行一次的 main() 入口，显著减少了无谓的阻塞时间，这是性能上的改进。
潜在问题：固定 1 秒的阻塞在每次启动时依然存在，且在服务快速启动的场景下白白浪费时间。
建议：改为短间隔（如 100ms）轮询，一旦 socket 可用立即返回，降低平均等待时间。

• 4.代码安全存在 2 个安全漏洞（高危1个，中危1个）✕

漏洞对比统计：新增漏洞 2 个，减少漏洞 0 个，持平 0 个
代码在处理无 DBus 模式的本地通信时，使用了不安全的固定路径 socket，且缺乏对后台进程启动状态的校验，存在本地劫持和提权风险。

• 安全漏洞1（【高危】）：不安全的临时文件/Socket路径 在 libs/linglong/src/linglong/cli/cli.cpp 的 getPkgMan() 函数中，使用了硬编码的 unix:path=/tmp/linglong-package-manager.socket。/tmp 目录默认全局可写，攻击者可预先创建指向敏感文件的符号链接。当 ll-package-manager 尝试绑定该 socket 时，可能引发拒绝服务，或攻击者伪造 socket 监听恶意响应，导致连接该 socket 的 root 权限 CLI 工具被欺骗执行恶意指令。——非常重要

• 安全漏洞2（【中危】）：进程启动缺乏状态校验 在 apps/ll-cli/src/main.cpp 的 main 函数中，通过 QProcess::startDetached 拉起服务后仅盲等 1 秒，未校验进程 PID 是否存活或 socket 是否由预期进程创建。在竞态窗口期，恶意用户可能抢占 socket，导致后续 CLI 操作连接至恶意后端，造成权限提升。——非常重要

• 建议：避免在 /tmp 下使用固定路径，应使用 abstract: 抽象命名空间 socket（Linux 特有，无文件系统路径），或放在受保护的目录（如 /run/linglong/）并设置严格权限；启动服务后应轮询检查 socket 文件的属主和权限，或通过 PID 文件验证进程身份。

■ 【改进建议代码示例】

// apps/ll-cli/src/main.cpp
    const bool peerMode = noDBusFlag->count() > 0;
    if (peerMode) {
        if (getuid() != 0) {
            LogE("--no-dbus should only be used by root user.");
            return -1;
        }

        // Start ll-package-manager --no-dbus first to initialize the repository
        QProcess::startDetached("sudo",
                                { "--user",
                                  LINGLONG_USERNAME,
                                  "--preserve-env=QT_FORCE_STDERR_LOGGING",
                                  "--preserve-env=QDBUS_DEBUG",
                                  LINGLONG_LIBEXEC_DIR "/ll-package-manager",
                                  "--no-dbus" });
        
        // Secure polling mechanism to wait for service readiness
        const QString secureSocketPath = "/run/linglong/linglong-package-manager.socket";
        constexpr int maxRetries = 20;
        constexpr int retryIntervalMs = 100;
        
        for (int i = 0; i < maxRetries; ++i) {
            QFileInfo fileInfo(secureSocketPath);
            // Verify socket exists and is owned by root to prevent TOCTOU attacks
            if (fileInfo.exists() && fileInfo.ownerId() == 0) {
                break;
            }
            QThread::msleep(retryIntervalMs);
        }
    }
    auto repo = linglong::repo::OSTreeRepo::loadFromPath(LINGLONG_ROOT);

// libs/linglong/src/linglong/cli/cli.cpp
    if (this->peerMode) {
        LogW("some subcommands will failed in --no-dbus mode.");
        // Use Linux abstract namespace to prevent symlink hijacking in /tmp
        const auto pkgManAddress = QString("unix:abstract=/linglong-package-manager");

        const auto &pkgManConn =
          QDBusConnection::connectToPeer(pkgManAddress, "ll-package-manager");

@dengbo11: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Full PR test history. Your PR dashboard.

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

The full list of commands accepted by this bot can be found here.